-
Notifications
You must be signed in to change notification settings - Fork 472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Enable portable flag for Docker container builds #1602
Conversation
Thanks for investigation, @ovaistariq! In my mind, options 1 and 0 are similar (with some case) - the compiler do as best as it can. But we need to can manual add this variable to compiler process. So, in case of #1146 a best way is change PORTABLE=znver1 (if CPU are a first family of EPYC arch). Change a default PORTABLE to 1 maybe a wrong case - as describe in rocksdb source - usually nothing to do; compiler default is typically the most general - but for hi-load system it's a wrong way, because we cant use an advanced CPU features and acceleration, 1 is a simple baseline CPU. Maybe we can detect a right CPU platform directly in x.py Python script and use it? |
In my opinion the Docker images are supposed to be portable for the platform they are built on. So an image built for the amd64 platform should be able to run on all amd64 machines. Secondly, the Docker image may be built on a machine with a different hardware spec then the machine the container is going to run on. For example in this case, the image is built on GitHub runners which are running Intel CPUs while the container is running on a machine with AMD CPUs. |
@aleksraiden This PR only changes the docker image PORTABLE value to 1. I think this makes a lot of sense since we cannot promise users are always running on the intel CPU like @ovaistariq mentioned. |
RocksDB PORTABLE was set to 0 after apache#1516 and it may return an illegal instruction error when running on the AMD platform. This PR fixes this issue by changing the default value of PORTABLE to 1 so that it can compile the rocksdb without platform special instructions.
RocksDB PORTABLE was set to 0 after #1516 and it may return an illegal instruction error when running on the AMD platform. This PR fixes this issue by changing the default value of PORTABLE to 1 so that it can compile the rocksdb without platform special instructions.
RocksDB PORTABLE was set to 0 after apache#1516 and it may return an illegal instruction error when running on the AMD platform. This PR fixes this issue by changing the default value of PORTABLE to 1 so that it can compile the rocksdb without platform special instructions.
Enable portable flag for Docker container builds
Fixes issue #1146